!ioctl: serialize nested ioctls outside of kernel by julianuziemblo · Pull Request #456 · phoenix-rtos/libphoenix

julianuziemblo · 2026-01-28T16:13:28Z

Description

Motivation and Context

Depends-On: phoenix-rtos-kernel:julianuziemblo/implement-siocethtool
Depends-On: phoenix-rtos-lwip:julianuziemblo/RTOS-1205
Depends-On: phoenix-rtos-tests:julianuziemblo/RTOS-1205

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality):
Serialization and deserialization is
Breaking change (fix or feature that would cause existing functionality to change):
Changed signature of ioctl syscall proxy function to:
int sys_ioctl(int fildes, unsigned long request, void *val, size_t size)
This was done to pass the size of the size of the whole structure to the kernel without affecting the request variable, so we can still match on it in the device server handling the ioctl. This eliminates the need to handle special cases in the kernel: userspace serializes the ioctl, and device server unpacks it.
Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

Already covered by automatic testing.
New test added: ioctl: add nested ioctl tests phoenix-rtos-tests#447
Tested by hand on: (list targets here).

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing linter checks and tests passed.
My changes generate no new compilation warnings for any of the targets.

Special treatment

This PR needs additional PRs to work:
I will merge this PR by myself when appropriate.

gemini-code-assist · 2026-01-28T16:13:49Z

Summary of Changes

Hello @julianuziemblo, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the ioctl system call mechanism by shifting the responsibility of handling complex argument serialization and deserialization from the kernel to userspace. This change aims to streamline the kernel's ioctl implementation, making it more generic and easier to maintain, while providing a robust and extensible framework for managing diverse ioctl argument types in the userspace library. The modification introduces a new helper module to manage this process, ensuring that structures with embedded pointers are correctly packed and unpacked before and after kernel calls.

Highlights

Centralized IOCTL Argument Handling: The core change involves moving the serialization and deserialization of complex ioctl arguments (especially those containing pointers) from kernel-side special-case handling to a new userspace helper library. This simplifies kernel logic and centralizes argument packing/unpacking.
Updated sys_ioctl Signature: The sys_ioctl proxy function's signature has been modified to int sys_ioctl(int fildes, unsigned long request, void *val, size_t size). This new size parameter allows passing the total size of the serialized structure to the kernel without altering the request variable, which is crucial for device server matching.
New IOCTL Helper Library: A new ioctl-helper.c and ioctl-helper.h pair has been introduced in unistd/ to manage the serialization, deserialization, and memory management for ioctl arguments. This library handles specific requests like SIOCGIFCONF, SIOCADDRT, and SIOCDELRT.
Refactored ioctl Wrapper: The userspace ioctl wrapper function in unistd/file.c now utilizes the new ioctl_serialize, sys_ioctl, and ioctl_deserialize functions to prepare and process ioctl arguments, ensuring proper handling of complex data structures.
IOCTL Macro Adjustments: Several ioctl-related macros in include/sys/ioctl.h have been updated, including IOCPARM_MASK, IOC_VOID, and IOC_DIRMASK, and a new convenience macro IOC_GET_PTR_FIELD has been added.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request refactors the handling of special-case ioctl commands by moving serialization logic from kernel-space to user-space. This is a solid architectural improvement that simplifies the kernel's role and centralizes complex data handling within libphoenix. The introduction of ioctl-helper.c provides a scalable way to manage ioctl commands with complex data structures containing pointers. The changes to the ioctl function signature and implementation are well-contained and improve correctness, particularly in handling variadic arguments. I have one high-severity security-related comment regarding a potential buffer over-read.

unistd/ioctl-helper.c

unistd/file.c

unistd/ioctl-helper.c

include/sys/ioctl.h

github-actions · 2026-02-12T17:44:00Z

Unit Test Results

9 553 tests 8 961 ✅ 52m 51s ⏱️
591 suites 592 💤
1 files 0 ❌

Results for commit d27bdb3.

♻️ This comment has been updated with latest results.

include/sys/ioctl.h

include/sys/sockios.h

unistd/ioctl-helper.c

include/sys/ioctl.h

There are some special cases where the input to the ioctl syscall is a structure with a pointer, which has to be detected and treated uniquely by the kernel. This leads to asymmetric behaviour (serialization in kernel, but deserialization in userspace) and code duplication. This change introduces simple packing and unpacking functions, which work only for the special cases, allocating memory for the whole request to be sent in the syscall and deallocating it when the syscall returns. This change also introduces function `ioctl_getPointerField` and convenience IOC_GET_PTR_FIELD macro, which allow the user to easily get the pointed-to values passed in by ioctl without knowing the inner serialization structure. JIRA: RTOS-1205

IOC* defines transferred to kernel to reduce code duplication JIRA: RTOS-1205

JIRA: RTOS-1205

include/sys/ioctl.h

julianuziemblo requested a review from Darchiv January 28, 2026 16:13

julianuziemblo mentioned this pull request Jan 28, 2026

ioctl: add nested ioctl tests phoenix-rtos/phoenix-rtos-tests#447

Merged

14 tasks

gemini-code-assist bot reviewed Jan 28, 2026

View reviewed changes

unistd/ioctl-helper.c Outdated Show resolved Hide resolved

This was referenced Jan 28, 2026

!ioctl: serialize nested ioctls outside of kernel phoenix-rtos/phoenix-rtos-kernel#656

Merged

port/sockets: adjust to special ioctl changes phoenix-rtos/phoenix-rtos-lwip#134

Merged

submodule update phoenix-rtos/phoenix-rtos-project#1539

Merged

Darchiv reviewed Feb 10, 2026

View reviewed changes

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch 2 times, most recently from f3e50ac to ac66aa1 Compare February 12, 2026 17:15

github-actions bot reviewed Feb 12, 2026

View reviewed changes

unistd/ioctl-helper.c Outdated Show resolved Hide resolved

include/sys/ioctl.h Outdated Show resolved Hide resolved

include/sys/ioctl.h Outdated Show resolved Hide resolved

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from ac66aa1 to cbd6d96 Compare February 12, 2026 17:19

julianuziemblo requested a review from Darchiv February 12, 2026 17:19

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from cbd6d96 to 21b189a Compare February 12, 2026 17:27

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from 21b189a to a368c14 Compare February 13, 2026 08:18

julianuziemblo changed the title ~~ioctl: serialize special-case ioctl-commands outside of kernel~~ ioctl: serialize nested ioctls outside of kernel Feb 13, 2026

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch 2 times, most recently from 5ee0d1d to 5886041 Compare February 18, 2026 10:51

github-actions bot reviewed Feb 18, 2026

View reviewed changes

include/sys/ioctl.h Outdated Show resolved Hide resolved

include/sys/ioctl.h Outdated Show resolved Hide resolved

Darchiv reviewed Feb 18, 2026

View reviewed changes

include/sys/sockios.h Outdated Show resolved Hide resolved

unistd/ioctl-helper.c Outdated Show resolved Hide resolved

unistd/ioctl-helper.c Show resolved Hide resolved

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from 5886041 to fd5bf33 Compare February 18, 2026 11:25

github-actions bot reviewed Feb 18, 2026

View reviewed changes

include/sys/ioctl.h Outdated Show resolved Hide resolved

include/sys/ioctl.h Outdated Show resolved Hide resolved

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from fd5bf33 to ea82cff Compare February 18, 2026 11:40

julianuziemblo requested a review from Darchiv February 18, 2026 16:37

julianuziemblo changed the title ~~ioctl: serialize nested ioctls outside of kernel~~ !ioctl: serialize nested ioctls outside of kernel Feb 25, 2026

Darchiv reviewed Mar 3, 2026

View reviewed changes

include/sys/ioctl.h Show resolved Hide resolved

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from ea82cff to 5e7c9cd Compare March 3, 2026 14:41

julianuziemblo requested a review from Darchiv March 3, 2026 16:29

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from 5e7c9cd to 5a110cd Compare March 4, 2026 09:11

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from 5a110cd to e06022f Compare March 4, 2026 09:23

julianuziemblo added 2 commits March 4, 2026 10:36

!include/sys/ioctl: include phoenix/ioctl.h for common IOC* defines

74582f1

IOC* defines transferred to kernel to reduce code duplication JIRA: RTOS-1205

include/sys/sockios: format

d27bdb3

JIRA: RTOS-1205

julianuziemblo force-pushed the julianuziemblo/RTOS-1205 branch from e06022f to d27bdb3 Compare March 4, 2026 09:36

github-actions bot reviewed Mar 4, 2026

View reviewed changes

include/sys/ioctl.h Show resolved Hide resolved

include/sys/ioctl.h Show resolved Hide resolved

Darchiv approved these changes Mar 4, 2026

View reviewed changes

julianuziemblo merged commit 47e2e9f into master Mar 12, 2026
46 checks passed

julianuziemblo deleted the julianuziemblo/RTOS-1205 branch March 12, 2026 10:38

Conversation

julianuziemblo commented Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Types of changes

How Has This Been Tested?

Checklist:

Special treatment

Uh oh!

gemini-code-assist bot commented Jan 28, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

julianuziemblo commented Jan 28, 2026 •

edited

Loading

github-actions bot commented Feb 12, 2026 •

edited

Loading